Skip to content

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jan 11, 2020

High-level summary of changes:

  • The syntax::node_count pass is moved into rustc_ast_passes. This works towards improving The rustc_query_impl crate is too big, which hurts compile times for the compiler itself #65031 by making compiling syntax go faster.

  • The syntax::{GLOBALS, with_globals, ..} business is consolidated into syntax::attr for cleaner code and future possible improvements.

  • The pretty printer loses its dependency on ParseSess, opting to use SourceMap & friends directly instead.

  • Some drive by cleanup of syntax::attr::HasAttr happens.

  • Builtin attribute logic (syntax::attr::builtin) + syntax::attr::allow_internal_unstable is moved into a new rustc_attr crate. More logic from syntax::attr should be moved into that crate over time. This also means that syntax loses all mentions of ParseSess, which enables the next point.

  • The pretty printer syntax::print is moved into a new crate rustc_ast_pretty.

  • rustc_session::node_id is moved back as syntax::node_id. As a result, syntax gets to drop dependencies on rustc_session (and implicitly rustc_target), rustc_error_codes, and rustc_errors. Moreover rustc_hir gets to drop its dependency on rustc_session as well. At this point, these crates are mostly "pure data crates", which is approaching a desirable end state.

    • We should consider renaming syntax to rustc_ast now.

@rust-highfive

This comment has been minimized.

Centril added a commit to Centril/rust that referenced this pull request Jan 11, 2020
…lacrum

Move more of `rustc::lint` into `rustc_lint`

Based on rust-lang#67806.

Here we try to consolidate more of the linting infra into `rustc::lint`. Some high-level notes:

- We now store an `Lrc<dyn Any + Send + Sync>` as opposed to `Lrc<LintStore>` in the `GlobalCtxt`. This enables us to avoid referring to the type, breaking a cyclic dependency, and so we can move things from `rustc::lint` to `rustc_lint`.

- `in_derive_expansion` is, and needs to, be moved as a method on `Span`.

- We reduce the number of ways on `tcx` to emit a lint so that the developer UX is more streamlined.

- `LintLevelsBuilder` is moved to `rustc_lint::levels`, leaving behind `LintLevelMap/Set` in a purified form due to current constraints (hopefully fixable in the future after rust-lang#68133).

- `struct_lint_level` is moved to `rustc::lint` due to current dependency constraints.

- `rustc::context` is moved to `rustc_lint::context`.

- The visitors in `rustc::lint` are moved to `rustc_lint::passes`.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 11, 2020
…lacrum

Move more of `rustc::lint` into `rustc_lint`

Based on rust-lang#67806.

Here we try to consolidate more of the linting infra into `rustc::lint`. Some high-level notes:

- We now store an `Lrc<dyn Any + Send + Sync>` as opposed to `Lrc<LintStore>` in the `GlobalCtxt`. This enables us to avoid referring to the type, breaking a cyclic dependency, and so we can move things from `rustc::lint` to `rustc_lint`.

- `in_derive_expansion` is, and needs to, be moved as a method on `Span`.

- We reduce the number of ways on `tcx` to emit a lint so that the developer UX is more streamlined.

- `LintLevelsBuilder` is moved to `rustc_lint::levels`, leaving behind `LintLevelMap/Set` in a purified form due to current constraints (hopefully fixable in the future after rust-lang#68133).

- `struct_lint_level` is moved to `rustc::lint` due to current dependency constraints.

- `rustc::lint::context` is moved to `rustc_lint::context`.

- The visitors in `rustc::lint` are moved to `rustc_lint::passes`.
Centril added a commit to Centril/rust that referenced this pull request Jan 12, 2020
…lacrum

Move more of `rustc::lint` into `rustc_lint`

Based on rust-lang#67806.

Here we try to consolidate more of the linting infra into `rustc::lint`. Some high-level notes:

- We now store an `Lrc<dyn Any + Send + Sync>` as opposed to `Lrc<LintStore>` in the `GlobalCtxt`. This enables us to avoid referring to the type, breaking a cyclic dependency, and so we can move things from `rustc::lint` to `rustc_lint`.

- `in_derive_expansion` is, and needs to, be moved as a method on `Span`.

- We reduce the number of ways on `tcx` to emit a lint so that the developer UX is more streamlined.

- `LintLevelsBuilder` is moved to `rustc_lint::levels`, leaving behind `LintLevelMap/Set` in a purified form due to current constraints (hopefully fixable in the future after rust-lang#68133).

- `struct_lint_level` is moved to `rustc::lint` due to current dependency constraints.

- `rustc::lint::context` is moved to `rustc_lint::context`.

- The visitors in `rustc::lint` are moved to `rustc_lint::passes`.
@bors

This comment has been minimized.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 12, 2020
@Centril Centril changed the title [WIP] Slimmer syntax Slimmer syntax Jan 17, 2020
@Centril
Copy link
Contributor Author

Centril commented Jan 17, 2020

r? @petrochenkov

@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

@petrochenkov
Copy link
Contributor

We should consider renaming syntax to rustc_ast now.

👍

@Centril
Copy link
Contributor Author

Centril commented Jan 17, 2020

We should consider renaming syntax to rustc_ast now.

👍

Cool; I'll follow up with this. :)

@Centril Centril added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 18, 2020
@Centril
Copy link
Contributor Author

Centril commented Jan 18, 2020

Addressed the review comments.

@rust-highfive

This comment has been minimized.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 18, 2020

📌 Commit 1fbab97f3d177c4f4d58f69a2b73d938740a2abc has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 18, 2020
@rust-highfive

This comment has been minimized.

@Centril

This comment has been minimized.

@bors

This comment has been minimized.

@Centril
Copy link
Contributor Author

Centril commented Feb 1, 2020

@bors r=petrochenkov

@bors
Copy link
Collaborator

bors commented Feb 1, 2020

📌 Commit 1a3141c has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 1, 2020
@bors
Copy link
Collaborator

bors commented Feb 1, 2020

⌛ Testing commit 1a3141c with merge 13db650...

bors added a commit that referenced this pull request Feb 1, 2020
Slimmer syntax

High-level summary of changes:

- The `syntax::node_count` pass is moved into `rustc_ast_passes`. This works towards improving #65031 by making compiling `syntax` go faster.

- The `syntax::{GLOBALS, with_globals, ..}` business is consolidated into `syntax::attr` for cleaner code and future possible improvements.

- The pretty printer loses its dependency on `ParseSess`, opting to use `SourceMap` & friends directly instead.

- Some drive by cleanup of `syntax::attr::HasAttr` happens.

- Builtin attribute logic (`syntax::attr::builtin`) + `syntax::attr::allow_internal_unstable` is moved into a new `rustc_attr` crate. More logic from `syntax::attr` should be moved into that crate over time. This also means that `syntax` loses all mentions of `ParseSess`, which enables the next point.

- The pretty printer `syntax::print` is moved into a new crate `rustc_ast_pretty`.

- `rustc_session::node_id` is moved back as `syntax::node_id`. As a result, `syntax` gets to drop dependencies on `rustc_session` (and implicitly `rustc_target`), `rustc_error_codes`, and `rustc_errors`. Moreover `rustc_hir` gets to drop its dependency on `rustc_session` as well. At this point, these crates are mostly "pure data crates", which is approaching a desirable end state.

  - We should consider renaming `syntax` to `rustc_ast` now.
@bors
Copy link
Collaborator

bors commented Feb 1, 2020

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing 13db650 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 1, 2020
@bors bors merged commit 1a3141c into rust-lang:master Feb 1, 2020
@rust-highfive
Copy link
Contributor

📣 Toolstate changed by #68133!

Tested on commit 13db650.
Direct link to PR: #68133

💔 clippy-driver on windows: test-fail → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra).
💔 clippy-driver on linux: test-fail → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Feb 1, 2020
Tested on commit rust-lang/rust@13db650.
Direct link to PR: <rust-lang/rust#68133>

💔 clippy-driver on windows: test-fail → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra).
💔 clippy-driver on linux: test-fail → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra).
JohnTitor added a commit to JohnTitor/rust-clippy that referenced this pull request Feb 1, 2020
@Centril Centril deleted the slimmer-syntax branch February 1, 2020 21:58
bors added a commit to rust-lang/rust-clippy that referenced this pull request Feb 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants